Skip to content

FlightData: add mass storage reboot action - #3756

Open
tridge wants to merge 1 commit into
ArduPilot:masterfrom
tridge:pr-reboot-mass-storage
Open

FlightData: add mass storage reboot action#3756
tridge wants to merge 1 commit into
ArduPilot:masterfrom
tridge:pr-reboot-mass-storage

Conversation

@tridge

@tridge tridge commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

to support this PR:
ArduPilot/ardupilot#34087
image

@tridge

tridge commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.
Full report: https://uav.tridgell.net/DevCallReviews/2026_08_30/devcall_pr_reviews.html#prMissionPlanner-3756

Reviewed at head cfc80d28c2. APPROVE — no blockers. What was checked: the new handler sits inside the standard "Are you sure" confirmation gate and follows the neighbouring disable/doCommand/re-enable/return pattern exactly; param1=5 matches the mass-storage action from the companion mavlink change implemented by ardupilot#34087 (linked in the body), and older firmware rejects it safely; the mid-enum insertion is safe in-tree (the action combo is built from Enum.GetNames, all dispatch compares by name, nothing persists numeric values). An independent Codex validation and a cold second review both ran.

One thing worth a line in the commit message: the TryGetValue refactor fixes a real bug on masterCustomActions is a Dictionary<string, Action<string>>, so master's CustomActions[CMB_action.Text] throws KeyNotFoundException on every built-in action click (only the global WinForms error-report handler catches it).

Non-blocking notes:

  1. FlightData.actions is public, so inserting mid-enum shifts later constants for any precompiled external plugin that baked them in. No in-tree numeric consumer exists, but appending the member at the end removes the edge for free (FlightData.cs:190).
  2. doCommand() special-cases PREFLIGHT_REBOOT_SHUTDOWN as fire-and-forget (never reads the ACK), so a rejection — old firmware, armed — is silent. Consistent with the existing Preflight_Reboot_Shutdown semantics; a "may be unsupported on this firmware" hint in the confirmation text would help while ardupilot#34087 is unreleased.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new FlightData “action” in MissionPlanner to reboot an ArduPilot flight controller into USB mass-storage mode (to support ArduPilot/ardupilot#34087), enabling faster access to the vehicle’s SD card via USB.

Changes:

  • Added a new actions.Reboot_Mass_Storage enum entry so it appears in the FlightData action list.
  • Implemented handling in BUTactiondo_Click to send MAV_CMD.PREFLIGHT_REBOOT_SHUTDOWN with the mass-storage reboot action parameter.
  • Refactored custom action lookup to use CustomActions.TryGetValue(...) instead of direct indexing.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread GCSViews/FlightData.cs
Comment on lines +1684 to +1688
if (CustomActions.TryGetValue(CMB_action.Text, out var customAction))
{
var customAction = CustomActions[CMB_action.Text];
if(customAction!=null)
try
{
try
{
customAction.Invoke(CMB_action.Text);
}
catch (Exception ex)
{
CustomMessageBox.Show(Strings.CommandFailed + "\n" + ex.ToString(), Strings.ERROR);
}
return;
customAction.Invoke(CMB_action.Text);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Fix the code for this review comment.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review comment.

Comment thread GCSViews/FlightData.cs
}
if (CMB_action.Text == actions.Reboot_Mass_Storage.ToString())
{
const float rebootToMassStorage = 5;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Fix the code for this review comment.

When a review comment includes a suggested change, apply the suggestion exactly.

Do not make changes beyond what is described in the linked review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants